Skip to content

stream: fix recursive WritableStream abort - #64825

Open
standard-Chan wants to merge 1 commit into
nodejs:mainfrom
standard-Chan:fix-writable-stream-recursive-abort
Open

stream: fix recursive WritableStream abort#64825
standard-Chan wants to merge 1 commit into
nodejs:mainfrom
standard-Chan:fix-writable-stream-recursive-abort

Conversation

@standard-Chan

@standard-Chan standard-Chan commented Jul 29, 2026

Copy link
Copy Markdown

Re-check the stream state after signaling the controller's abort signal, as required by steps 3 and 4 of the WritableStreamAbort algorithm.

Refs: https://streams.spec.whatwg.org/#writable-stream-abort

Also, reject close promise caches only when their promises are still pending. A cache lazily materialized after the stream becomes errored may already contain a rejected promise and does not have a callable reject method.

Remove the expected-failure entry for the recursive abort WPT.

Testing

$ ./node test/wpt/test-streams.js writable-streams/aborting.any.js

Files: 1/1 ran, 1 passed, 0 skipped, 0 expected failures, 0 unexpected failures, 0 unexpected passes
Subtests: 65 passed, 0 skipped, 0 expected failures, 0 unexpected failures, 0 unexpected passes

Recheck the stream state after signaling abort, as required by steps 3
and 4 of the WritableStreamAbort algorithm.

Only reject pending close promise caches, as a cache materialized after
the stream becomes errored may already contain a rejected promise.

This makes the recursive abort WPT pass and removes its expected-failure
entry.

Refs: https://streams.spec.whatwg.org/#writable-stream-abort
Signed-off-by: Jeong SeokChan <starp321@naver.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/web-standards

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Jul 29, 2026
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.15%. Comparing base (f8f8aa2) to head (84727d9).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #64825   +/-   ##
=======================================
  Coverage   90.14%   90.15%           
=======================================
  Files         746      746           
  Lines      242818   242821    +3     
  Branches    45765    45765           
=======================================
+ Hits       218884   218904   +20     
- Misses      15417    15419    +2     
+ Partials     8517     8498   -19     
Files with missing lines Coverage Δ
lib/internal/webstreams/writablestream.js 99.50% <100.00%> (+<0.01%) ⬆️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@standard-Chan

Copy link
Copy Markdown
Author

The build-tarball job was canceled because exceeding the 15-minute execution limit. Could someone please rerun the failed job?

@daeyeon daeyeon added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 1, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 1, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants